Skip to content

fix: prevent silent None return in run_with_retry when max_retries < 1#198

Open
amathxbt wants to merge 2 commits intoOpenGradient:mainfrom
amathxbt:fix/run-with-retry-silent-none-return
Open

fix: prevent silent None return in run_with_retry when max_retries < 1#198
amathxbt wants to merge 2 commits intoOpenGradient:mainfrom
amathxbt:fix/run-with-retry-silent-none-return

Conversation

@amathxbt
Copy link
Contributor

@amathxbt amathxbt commented Mar 21, 2026

Bug

run_with_retry in _utils.py silently returns None when max_retries resolves to 0 or any value less than 1.

# Calling with max_retries=0
result = run_with_retry(my_tx_fn, max_retries=0)
# range(0) is empty → loop never executes → function falls off the end → returns None

All callers assign the result directly:

result: InferenceResult = run_with_retry(execute_transaction, max_retries)
contract_address: str = run_with_retry(deploy_transaction)

A None return causes a downstream AttributeError or TypeError that is confusing to debug — the real mistake (passing max_retries=0) is far removed from where the crash occurs.

Root Cause

effective_retries = max_retries if max_retries is not None else DEFAULT_MAX_RETRY

for attempt in range(effective_retries):  # range(0) → empty → function returns None
    ...

No validation of effective_retries before entering the loop.

Fix

Two-layer defense:

  1. Early validation — raise ValueError immediately if effective_retries < 1:
if effective_retries < 1:
    raise ValueError(f"max_retries must be at least 1, got {effective_retries}")
  1. Post-loop safety net — add an explicit RuntimeError after the loop (theoretically unreachable, but prevents any future refactor from silently returning None):
raise RuntimeError(f"run_with_retry exhausted {effective_retries} attempts without returning or raising")

Impact

  • Raises a clear ValueError at the call site instead of a cryptic NoneType error downstream
  • The loop logic and existing retry behaviour are completely unchanged
  • Zero risk of regression for all callers passing valid max_retries values

@amathxbt
fix: prevent silent None return in run_with_retry when max_retries < 1
29d7f2d 
Previously, calling run_with_retry with max_retries=0 (or any value
that resolves to < 1) caused range(0) to produce an empty loop body,
silently returning None instead of executing the transaction.

Downstream callers assign the result directly (e.g. result = run_with_retry(...))
so a None return causes a confusing AttributeError or TypeError far from
the actual source of the problem.

Fix:
- Add explicit ValueError guard when effective_retries < 1
- Add unreachable-but-explicit RuntimeError after loop as a safety net
@adambalogh
Copy link
Collaborator

adambalogh commented Mar 23, 2026

Hi thanks for the contribution! Would you mind merging from main? Thank you

@amathxbt amathxbt mentioned this pull request Mar 23, 2026
Merged
@amathxbt
fix: restore and update _utils.py with proper merge from main
71c5004 
The PR branch had an empty _utils.py due to a merge issue.
This commit:
1. Restores all existing code from upstream main
2. Adds ValueError guard when effective_retries < 1
3. Adds RuntimeError safety net after the retry loop

Fixes the silent None return bug as described in the PR.
@amathxbt
Copy link
Contributor Author

amathxbt commented Mar 23, 2026 •
edited
Loading

Hey @adambalogh — thanks for the feedback! Done

Here is what was done to address your request:

Merged from main

The fork has been synced with the latest upstream main (fast-forwarded to ede8b8ca) and main has been merged into this PR branch.

Root cause found & fixed

During the merge, it was discovered that _utils.py on the PR branch had been accidentally emptied (contained only a single newline). This has been corrected — the file now includes:

  1. All existing code from the current upstream main (including the result: dict type annotation in get_abi and the updated RuntimeError in the retry loop)
  2. The ValueError guard before the loop — raises immediately when max_retries < 1: 
    if effective_retries < 1:
    raise ValueError(f"max_retries must be at least 1, got {effective_retries}")
  3. The post-loop RuntimeError safety net — prevents any future silent None return: 
    raise RuntimeError(f"run_with_retry exhausted {effective_retries} attempts without returning or raising")

Commit

71c50049 — fix: restore and update _utils.py with proper merge from main

Happy to make any further changes if needed. Let me know!

@amathxbt
Copy link
Contributor Author

amathxbt commented Mar 25, 2026

@adambalogh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@amathxbt @adambalogh